-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(openapi): add base url #308
Conversation
|
✅ Deploy Preview for openpayments-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -112,7 +112,7 @@ paths: | |||
New Incoming Payment for $25: | |||
value: | |||
id: 'https://openpayments.guide/incoming-payments/08394f02-7b7b-45e2-b645-51d04e7c330c' | |||
walletAddress: 'https://openpayments.guide/alice/' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe walletAddress
values should still have /alice
. That seems like the case to me but I could be wrong @mkurapov / @sabineschaller
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the wallet address remains the same, just the baseUrl changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this and similar are now https://rafiki.money/alice/
.changeset/loud-snakes-melt.md
Outdated
@@ -0,0 +1,5 @@ | |||
--- | |||
'@interledger/open-payments': major |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this constitutes a major change. In fact, I'm not sure this changes anything in the open-payments
package itself at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
openapi/resource-server.yaml
Outdated
@@ -32,8 +32,8 @@ info: | |||
contact: | |||
email: [email protected] | |||
servers: | |||
- url: '/' | |||
description: 'Server for wallet address subresources or Connection resources (ie https://openpayments.guide/alice)' | |||
- url: 'https://openpayments/' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if I have a preference, but rafiki.money
might be nice to have if we are going to include some code snippets in the documentation that do use rafiki.money
. I would connect with Radu /Melissa/Mohammed for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will change to rafiki.money. Melissa confirmed that there will be code snippets in the docs that are intended to be used with rafiki.money. https://github.com/interledger/open-payments-snippets She also noted the openpayments.guide domain might change as well but in any case it seems like the rafiki.money url will be more useful.
@@ -112,7 +112,7 @@ paths: | |||
New Incoming Payment for $25: | |||
value: | |||
id: 'https://openpayments.guide/incoming-payments/08394f02-7b7b-45e2-b645-51d04e7c330c' | |||
walletAddress: 'https://openpayments.guide/alice/' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the wallet address remains the same, just the baseUrl changes
Changes proposed in this pull request
rafiki.money
as base pathContext
baseUrl
to OP spec #303The issues suggests maybe using rafiki.money as the base url but I opted for openpayments.guide because that's what it originally was. https://github.com/interledger/open-payments/blob/5d39b2ef5f202e6378555b790730f9465ecd6f97/openapi/resource-server.yaml#L35C3-L39 This makes the base url consistent with existing examples.